Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

RFC for branch management #1370

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mattmattmatt
Copy link
Contributor

Formal proposal of improvements to branch management.

🎨 Pretty markdown version.

Related discussion and design ideating are available in #556.

Implementation phases

  1. Extract "Create branch" from current branch-menu-view into command palette.
  2. Replace branch switching in branch-menu-view with command palette, removing branch-menu-view.
  3. Add branch renaming feature.
  4. Add branch deleting feature.

@annthurium @kuychaco @simurai @smashwilson

@winstliu winstliu added the feature request Propose new features or design label Mar 31, 2018
@simurai
Copy link
Contributor

simurai commented Apr 4, 2018

Everything proposed would be a great improvement to the current branch tooltip. 👍

Is this a worthwhile undertaking when proposals like #556 exist?

A concern @kuychaco mentioned: If we would do first this RFC, and then later move branch management to a panel #556, some people might prefer to keep using the command palette and get 😖.

Something I like about the command palette UI: Once you're done, the dialog closes automatically and the rest of the UI is still the same. With a panel/dock item, you have to manually switch back to, for example, the tree-view. Seeing the list of branches isn't that beneficial while you code.

On the other hand, having a panel/dock item gives us the freedom to add any custom UI we want. So maybe the question is: When does the command palette UI reach its limits? And does that happen anytime soon-ish?

What kind of warnings should branch deletion trigger?

Maybe a branch with merged commits can be deleted without confirmation? Since nothing is lost. And also have an icon or different color, so it's easy to see which branches aren't really needed anymore.


Some more open questions:

1. Sorting of the branch list

Should the list of local branches be sorted in a special way (other than alphabetically)?

  • Show master (default branch) first. Since it's the branch you most likely want to create a new branch from.
  • Then show the branches chronologically so that most recent branches are at the top. Usually only a few branches are "active", the rest are stale and might never get worked on again.

2. "All in one" UI

Another thing I wonder, should there be an additional github:branch command that shows the command palette where all other commands are possible. Maybe the other commands wouldn't even be needed. Somewhere I think we talked already about it, but it would work like this:

checkout-branch

  • Once github:branch is ran, it shows a list of your local branches. Just like github:checkout-branch.
  • Now you can use your arrow keys and press enter to do a checkout.
  • Or first start typing to filter the branch list.

create-new-branch

  • Once you start to type in the input, the first item in the list says something like: branchname "Press enter to create new branch"
  • It wouldn't show when you type an already existing branch name.

delete-branch

  • Once a branch is selected in the list, pressing [backspace/delete] would delete the branch.

rename-branch

  • Once a branch is selected in the list, pressing [f2] would allow to rename the branch.

Some benefits:

  • There is only one command/keybinding to remember.
  • No need to switch commands. You probably realize to clean up your branch list while you try to checkout another branch. "Damn, this list is getting long, I should clean it up!" But instead of having to run another command, you can just start to press backspace/delete and do it right then when it comes to mind.
  • When you try to create a new branch, you might see that it already exists, so it's easy to just slightly change the name without getting a "branch name already exists" warning.

Concerns:

Harder to discover. So it probably would need some extra hints, maybe at the bottom of the list. Like

Type name + [enter] to create a new branch, [backspace] to delete, [f2] to rename.

Each list item could also have a context menu with [Delete] and [Rename] as an extra option.

@kuychaco kuychaco self-requested a review April 14, 2018 02:00
Copy link
Contributor

@smashwilson smashwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more that I think about this the more that I prefer it to what we've got in #556. 👍

I was initially hesitant because I prefer to avoid modal interactions when I can in favor of more continuous representations of state. But I really like how this will scale to a large number of branches and the consistency with other Atom UI.

This does not give us an obvious way to add merge or rebase later... but now I think those belong in the log view, anyway, where you have more context.


## Motivation

Current branch management is incomplete (e.g. no deleting of branches) and not consistent with other conventions found in the Atom UI for comparable actions. It is hard to switch or create new branches with just keyboard input.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point about keyboard input here.

- Create new branch (`github:create-new-branch`)
- Checkout branch (`github:checkout-branch`)
- Delete branch (`github:delete-branch`)
- Rename branch (`github:rename-branch`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I second @simurai's suggestion about collapsing these into a single command, or at least checkout and create. That'd be consistent with the way that github.com does it:

screen shot 2018-04-24 at 9 51 58 pm

I think it'd be nice to not have to lose your search and place in the list to do a bunch of branch-related tasks at once.


#### `github:checkout-branch`

Executing `github:checkout-branch` opens a filterable command palette list of local branches, similar to the list of available grammars when executing `grammar-selector:show`. Selecting a branch from that list switches to that branch.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you feel about including remote tracking branches in the filter list? We could style or decorate them slightly differently as a visual cue, and then create a new local branch that tracks them like git checkout -b does. Might be a nice way to fill in some missing functionality from the existing branch selector.


#### `github:delete-branch`

Executing `github:delete-branch` opens a filterable command palette list of local branches, the same as executing `github:checkout-branch`. Selecting a branch from that list will show a prompt asking for confirmation to delete _[named branch_], including information whether that branch's merge status (e.g. _'test3' has been merged to 'refs/remotes/origin/test3', but not yet merged to HEAD._).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like having the extra merge status in the list here. It's a nice way to give you a little more context about the action you're about to take.

I'm not sure what this looks like in a world where the branch commands are unified. Any ideas, @simurai? Maybe we always show the merge status if there is one... ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a different icon?

image

The merged branch icon is used for merged pull requests, but maybe ok in this context.

Maybe even less contrast to signal: "This branch can probably be deleted".

image

@mattmattmatt
Copy link
Contributor Author

Thanks for the feedback so far, folks, that's great input. I like the idea of having only one branch list and then ways to execute all possible actions based on that.
As you say, it has both the benefit of only having to remember (and "reserving") one keyboard shortcut, and it encourages low-friction branch cleanup while one is noticing how long the branch list gets.

I would also consider showing remote branches in that list, but probably below all local branches. I think combining that with @simurai's suggestion of sorting branches by master, then recency, then remote branches, would make sense. Together with the icons proposed above (and a slightly different one for remote branches), this should make it easy to visually filter even a long list.

@simurai
Copy link
Contributor

simurai commented Apr 27, 2018

@mattmattmatt I would also consider showing remote branches in that list

💯 That would be awesome. I'll make some mockups to explore how it could look. Maybe have some divider between local + remote.

@simurai
Copy link
Contributor

simurai commented Apr 30, 2018

Here some mockups:

branches

Remote default branch

A repo's default branch always shows up at the top (minus when filtering). Note that this is the default branch on the remote (origin). This makes it possible to quickly switch to the default branch and get the latest changes without having to fetch + pull.

screen shot 2018-04-30 at 2 29 20 pm

In the above example, you can switch to the local master branch if you need to, but that's mostly to go back in history and test a certain commit. Otherwise always having an up-to-date master is probably preferred.

Local vs remote

The list has "gaps" to divide local and remote branches. In addition, remote branches show the remote name (e.g. origin).

screen shot 2018-04-30 at 2 31 46 pm

Branches are sorted by most recent changes and shown in these groups:

origin/master
----
local branches
----
remote origin branches
----
remote A branches
----
remote B branches

Open question: Should local branches show the remote tracking branch? Maybe when you select a local branch it could show it like:

screen shot 2018-04-30 at 3 26 39 pm

Or something a bit more subtle like showing a tooltip on hover. In the future we might can let users switch the remote tracking branch as well.

Current branch

The current branch has a border on the left.

screen shot 2018-04-30 at 2 25 20 pm

Filtering

To filter the list, you can start typing.

filter

Create a branch

If you type long enough, the "Create branch" option will probably be the only one left and thus selected. Pressing now enter creates the new branch.

create branch

This behavior might be a bit harder to to discover than having a dedicated "Create branch" button. We can add a hint at the bottom. Also, .com already uses it, so it wouldn't be completely new.

Open question: Should there be an option to directly create a new branch from origin/master? Similar to Desktop:

screen shot 2018-04-27 at 6 34 42 pm

Sometimes having to do an extra step might be ok because you don't have to read a dialog every time and make a choice.

Exact match

If you type a name that matches a local branch, the "Create branch" won't be shown.

exact match

Rename

When selecting a branch and pressing f2, the branch can be renamed:

rename

If the name already exists, the message changes to:

name exists

Delete

When selecting a branch and pressing delete, the branch gets deleted and removed from the list.

In case the branch contains un-merged commits, a confirmation dialog will be shown:

delete

Open question: Should the confirmation dialog have a "don't show this dialog again" option?

Also, about showing the merge status of a branch. Feedback from a recent design review was that having a different icon and greyed out branch name isn't very obvious what it means. Even when adding a label/badge saying merged, it might still be confusing. "Merged with what?", "Is this a PR?".

screen shot 2018-04-30 at 5 03 50 pm

I'll post some more ideas in the next comment.

@simurai
Copy link
Contributor

simurai commented Apr 30, 2018

"Is this a PR?"

The branches overview on .com shows some additional information, like if a branch has an associated PR and that PR's state.

screen shot 2018-04-30 at 5 25 56 pm

Here a possible mockup:

branches plus

It adds:

  • CI status
    • Gives you a hint what to expect before checking out a branch.
  • PR with #, status icon and avatar.
    • Makes it easier to find a certain branch. If someone says: "I just opened a PR for it". You can guess it is at the top of the remote, still green and has the person's avatar. You can also search/filter for a PR #, the author's name or maybe even with state, like is:open.
    • If a local branch has a merged PR, you probably don't need to keep it anymore and can delete it. If the PR is closed, but not merged, you might want to keep it locally to cherry pick from.

The above additions could be added as "nice to have" with less priority.

@donokuda
Copy link

donokuda commented May 2, 2018

Oh wow, really love the direction this is heading towards, @simurai + @mattmattmatt! 😍Had a couple of thoughts 💭:

  • Really like the separation between each remote and deemphasizing the remote name. 👍 It made the list really easy to parse and I might consider adopting this grouping method to GitHub Desktop 😉
  • If space permits, it might make sense to try to incorporate the full status of the pull request in words (i.e., Open, Merge, Closed, etc.)
  • I wonder how obvious is it that the avatar shown is the author of the pull request. Since there really isn't a label, I could also assume it represents the current contributors for the branch, assignee, reviewers, etc. I would consider testing this with other people to see if it's clear what the avatar represents.

Should local branches show the remote tracking branch?
...
Or something a bit more subtle like showing a tooltip on hover. In the future we might can let users switch the remote tracking branch as well.

I would find it helpful to understand which remote branch is a local branch tracking. A tooltip or something would make more sense here, given that there isn't a lot of space with both the local and remote branch name.

@simurai
Copy link
Contributor

simurai commented May 3, 2018

If space permits, it might make sense to try to incorporate the full status of the pull request in words (i.e., Open, Merge, Closed, etc.)

Space is a bit unpredictable. The One themes have a width of 600px, the classic Atom themes 500px. Other themes may be different. It also depends a bit on a theme's font-size. But yeah, having the PR state written out, might still be fine. 👍

I wonder how obvious is it that the avatar shown is the author of the pull request. Since there really isn't a label, I could also assume it represents the current contributors for the branch, assignee, reviewers, etc.

I was wondering about that too. Also because .com shows who is the last committer of a branch. Like Updated 2 weeks ago by username. One way to make it more clear is to also show the PR title after the avatar. But then it kinda needs it's own line.

Or maybe [PR state] #PR-nr by [avatar]? "by" would hint that it's the author? Would look like:

branches plus 2

@simurai simurai self-requested a review May 15, 2018 06:23
@smashwilson smashwilson mentioned this pull request Jun 21, 2018
20 tasks
@simurai simurai removed their request for review March 1, 2019 01:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature request Propose new features or design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants